Repository navigation
Sample-transfer start: flush due deltas only, not the sequencer tick - #1210
Conversation
amy_parse_transfer_layer_message() called the full amy_execute_deltas() before pcm_load() (4a48092), so that a queued reset lands first -- amy_reset_oscs() unloads all PCM presets, and would otherwise wipe the new sample when it played. But that call runs on the sending thread, and amy_execute_deltas() also advances the sequencer (an unlocked read-modify-write of next_amy_tick_us that races the render thread's own tick, and can run the external sequencer hook off the audio thread) and polls CV inputs. 2b2876b fixed the same thing for patch loads. Add amy_settle_deltas(): the render lock plus flush_due_deltas(), nothing else, safe from any thread. Use it for the sample-transfer start, the patch-load path (which open-coded the same three lines), and amy_execute_deltas() itself. Fixes #1207. make ctest passes; make test gives the same 90 pass / 43 fail as before here, with TestLoadSample (reset, then load a sample) matching its reference; ASan patch-reload stress 5/5 clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UW4jqUQ4Xc1sdoqzUPL2YV
🎛️ AMY HW CI (AMYboard bench)Flashed this PR's AMY (LoadTestChord: 6-voice Juno ✅ PASS — the bench ran the test to completion.
Full chord settled render μs: 2609 (was 2605, Δ +0.2%) (peak 2612, 39 samples) ⬇️ Artifacts: serial log · load trace · report Self-hosted bench (amyboardci). FAIL means only that the test could not run — the load values are informational, with no threshold and no audio compare. See |
⛓️ tulipcc integration PR openedThis merge was pinned into tulipcc for full-system CI: shorepine/tulipcc#1385 Test it there and merge that PR to move tulipcc onto this AMY. |
3 participants
Fixes #1207.
The problem. When a sample load starts (
z<preset>,<length>,…),amy_parse_transfer_layer_message()insrc/parse.ccalls the fullamy_execute_deltas()beforepcm_load(). That call runs on whichever thread sent the message, and besides flushing due deltas it:sequencer_check_and_fill()). That's an unlocked read-modify-write ofnext_amy_tick_us, which races the render thread's own tick, and it can run the external sequencer hook off the audio thread.update_external_cv_in()).2b2876b fixed the same thing for patch loads.
Why the flush is there at all. It came in with 4a48092 ("Execute pending resets before loading Memory PCM"). A queued reset runs
amy_reset_oscs(), which callspcm_unload_all_presets(). Without the flush, a sample loaded right afteramy.reset()is wiped when the reset finally plays. That only needs the due deltas to run, not the sequencer tick or the CV poll.The change:
amy_settle_deltas()(src/amy.c, declared inamy.h): the render lock (Render lock: keep a patch load's flush out of an in-progress render #1205) plusflush_due_deltas(), nothing else. Safe from any thread.src/parse.c: the sample-transfer start calls it instead ofamy_execute_deltas(), with a comment saying why the flush is needed and why it mustn't advance the sequencer there.amy_execute_deltas()use the same helper instead of each spelling out the lock-and-flush. Behaviour there is unchanged.Testing
make ctestpasses.make testgives the same 90 pass / 43 fail asmainin my environment; the failures are small numeric differences against the reference audio.TestLoadSample, which resets and then loads a sample, matches its reference.🤖 Generated with Claude Code
https://claude.ai/code/session_01UW4jqUQ4Xc1sdoqzUPL2YV
Generated by Claude Code